Add mass to MDAnalysis base units - #5439
Conversation
Documentation build overview
19 files changed ·
|
|
The failing checks appear unrelated to this PR. The GitHub Actions failures are in |
|
Cycling PR to get CI to run again. |
|
Can you merge the latest develope (or rebase)? There may have been updates addressing CI issues. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #5439 +/- ##
========================================
Coverage 93.87% 93.87%
========================================
Files 182 182
Lines 22522 22522
Branches 3206 3206
========================================
Hits 21143 21143
Misses 917 917
Partials 462 462 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
orbeckst
left a comment
orbeckst
left a comment
There was a problem hiding this comment.
Please add an entry to CHANGELOG (even though its docs, it does have an open issue associated with it and it clarifies some basic underlying assumptions). Also add yourself to AUTHORS.
Can you add a sentence after the base units table to mention CODATA2010 explicitly? See suggestion.
|
@sici17 please explain briefly how you used AI for this PR. |
|
Hi Orbecks, thanks a lot for the review. I rebased the branch onto the latest develop, added the CHANGELOG entry and my name to AUTHORS, and added the requested CODATA 2010 clarification. About the AI usage, I can say more specificallly that I used ChatGPT/Codex to help understand the issue, locate the relevant definitions inside the code and the project, start and locate the tests and inspect the logs. I personally reviewed and applied all the changes. I made sure to understand everything I was doing before actually doing it. I’ll verify the rendered documentation once the new Read the Docs build completes. |
orbeckst
left a comment
orbeckst
left a comment
There was a problem hiding this comment.
Thanks, just a minor CHANGELOG rearrangement is needed.
| * Mass (u) has been added to MDAnalysis base units and | ||
| clarified that physical constants use CODATA 2010 values | ||
| (Issue #3944, PR #5439) |
There was a problem hiding this comment.
We add new entries at the top. Please move to directly under Fixes
2 participants
Fixes #3944
Changes made in this Pull Request:
massto theMDAnalysis.unitsbase-units documentation table."mass": "u"toMDANALYSIS_BASE_UNITS.I did not update
package/CHANGELOGbecause this is a small documentation/base-units table fix.The SI value used for
uis1.66053892103219 × 10^-27 kg, which is consistent with the CODATA 2010 constants already used inMDAnalysis.units.Note that the User Guide currently lists a newer value for
u(1.66053906660(50) × 10^-27 kg). This PR intentionally keeps theMDAnalysis.unitsAPI documentation consistent with the existing CODATA 2010 values used by the module, following the maintainer guidance in the issue.LLM / AI generated code disclosure
LLMs or other AI-powered tools (beyond simple IDE use cases) were used in this contribution: yes
PR Checklist
package/CHANGELOGfile updated?package/AUTHORS? (If it is not, add it!)Developers Certificate of Origin
I certify that I can submit this code contribution as described in the Developer Certificate of Origin, under the MDAnalysis LICENSE.